feat(glue-alpha): pair workerType and numberOfWorkers into a required workerConfiguration - #38576
feat(glue-alpha): pair workerType and numberOfWorkers into a required workerConfiguration#38576otaviomacedo wants to merge 2 commits into
Conversation
… workerConfiguration
Worker sizing was modeled as two independent optional props (workerType,
numberOfWorkers) on the base JobProps. This let subtypes that cannot honor them
inherit them (PythonShellJob dropped them silently; RayJob accepted workerType
only to reject anything but Z.2X), and left the pair's co-dependency validated in
only two of the six Spark job types, with two different error codes.
Move the pair onto a WorkerConfiguration value object exposed as the optional
workerConfiguration prop on SparkJobProps, so a Spark job can never be given one
field without the other -- the illegal state is now a compile error instead of a
synth-time throw, and the duplicated/divergent validation is removed. Remove
workerType/numberOfWorkers from the base JobProps: PythonShellJob (DPU-sized via
maxCapacity) no longer surfaces them, and RayJob keeps its own numberOfWorkers
while dropping the fixed-value workerType.
Addresses the two workerType/numberOfWorkers findings from the aws-glue-alpha
pre-GA API review.
BREAKING CHANGE: workerType and numberOfWorkers are no longer top-level job props.
For Spark jobs, pass them together via workerConfiguration: { workerType,
numberOfWorkers }. PythonShellJob no longer accepts them (it is sized by
maxCapacity). RayJob no longer accepts workerType (it is fixed to Z.2X).
|
PRs without a linked issue will receive lower priority for review and merging. Please update the description to follow the PR template and include a line like |
otaviomacedo
added
the
pr-linter/exempt-integ-test
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
| readonly numberOfWorkers?: number; | ||
|
|
||
| /** | ||
| * Worker Type (optional) | ||
| * Type of Worker for Glue to use during job execution | ||
| * Enum options: Standard, G_1X, G_2X, G_025X. G_4X, G_8X, Z_2X | ||
| * | ||
| * @default WorkerType.G_1X | ||
| */ | ||
| readonly workerType?: WorkerType; |
| * | ||
| * @default - the job runs with the G_1X worker type and 10 workers. | ||
| */ | ||
| readonly workerConfiguration?: WorkerConfiguration; |
There was a problem hiding this comment.
By moving it to SparkJobProps instead of leaving it to JobProps means this property will not be anymore in PythonShellJobProps, is that intended?
There was a problem hiding this comment.
Yes, but PythonShellJobProps never reads them. What controls its capacity is maxCapacity.
| * configuration requires both values, so a Spark job can never be given one | ||
| * without the other. | ||
| */ | ||
| export interface WorkerConfiguration { |
There was a problem hiding this comment.
I'm always confused when nesting vs flat for me this against guideline, can you explain to me why did you go with this choice and also update the guideline for other to understand?
There was a problem hiding this comment.
By default, we should prefer flat interfaces, to make it more ergonomic for other jsii languages, such as Java. So, if it's just to organize the fields more nicely, it's not worth it. But here, the nested interface serves another ergonomic purpose: to make sure that invalid states are unrepresentable. In this case, workerType and numberOfWorkers must either both be set or neither. If they are two independent optional props in a flat interface, nothing prevents you from setting one and not the other. If they are required properties in a nested interface, you can't do that.
aws-cdk-automation
added
the
pr/needs-maintainer-review
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Merge Queue Status
This pull request spent 3 minutes 59 seconds in the queue, with no time running CI. Waiting for
All conditions
ReasonThe merge conditions cannot be satisfied due to failing checks Failing checks:
HintYou may have to fix your CI before adding the pull request to the queue again. Tick the box to put this pull request back in the merge queue (same as
|
3 participants
Worker sizing was modeled as two independent optional props (
workerType,numberOfWorkers) on the baseJobProps. This let subtypes that cannot honor them inherit them (PythonShellJobdropped them silently;RayJobacceptedworkerTypeonly to reject anything butZ.2X), and left the pair's co-dependency validated in only two of the six Spark job types, with two different error codes.Move the pair onto a
WorkerConfigurationvalue object exposed as the optionalworkerConfigurationprop onSparkJobProps, so a Spark job can never be given one field without the other -- the illegal state is now a compile error instead of a synth-time throw, and the duplicated/divergent validation is removed. RemoveworkerType/numberOfWorkersfrom the baseJobProps:PythonShellJob(DPU-sized viamaxCapacity) no longer surfaces them, andRayJobkeeps its ownnumberOfWorkerswhile dropping the fixed-valueworkerType.BREAKING CHANGE:
workerTypeandnumberOfWorkersare no longer top-level job props. For Spark jobs, pass them together viaworkerConfiguration: { workerType, numberOfWorkers }.PythonShellJobno longer accepts them (it is sized bymaxCapacity).RayJobno longer acceptsworkerType(it is fixed toZ.2X).By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license